[feat] Scaffold the mobile web app at /m (1/12) - #5679
Conversation
dosubot
Bot
added
size:XL
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe pull request adds a mobile Next.js workspace under ChangesMobile application foundation
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.agents/skills/mobile-app-structure/SKILL.md (1)
52-53: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winRun filtered mobile commands from
web/.Both skill files omit the workspace directory for
pnpm --filtercommands. Add the directory in both instructions.
.agents/skills/mobile-app-structure/SKILL.md#L52-L53: Prefix both filtered commands withFrom web/, or use an explicit--dir webform..agents/skills/mobile-shadcn-conventions/SKILL.md#L35-L35: Prefix token generation withFrom web/; keepweb/mobile/for the shadcn CLI instructions.Based on learnings, run mobile commands from the documented
web/workspace. As per path instructions,web/mobile/AGENTS.mddocuments commands fromweb/.Source: Learnings
🧹 Nitpick comments (2)
.agents/skills/mobile-app-structure/SKILL.md (1)
39-42: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winClarify the data boundary.
This rule permits “thin fetchers in
lib/”.web/mobile/AGENTS.mdsays data and state come from@agenta/*packages only. Confirm that alib/fetcher may only wrap an@agenta/*client, then state that restriction here.Proposed clarification
- components get data via hooks from `@agenta/*` packages or thin fetchers in `lib/`. + components get data via hooks from `@agenta/*` packages. Thin fetchers in + `lib/` may only wrap those package clients.As per path instructions,
web/mobile/AGENTS.mdlimits data and state to@agenta/*packages.web/mobile/eslint.config.mjs (1)
22-25: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse the plugin's recommended preset instead of two manually-picked rules.
eslint-plugin-react-hooks7.1.1 bundles compiler-aware rules (purity,refs,set-state-in-effect,immutability,static-components, and more) in itsrecommended/recommended-latestflat presets, enabled by default. Manually setting onlyrules-of-hooksandexhaustive-depsskips all of these.Spread the preset so the mobile app gets full React 19/compiler-aware Hooks linting:
♻️ Proposed refactor
const config = [ eslint.configs.recommended, ...tseslint.configs.recommended, ...tseslint.configs.stylistic, + reactHooks.configs.flat.recommended, { plugins: { import: importPlugin, "react-hooks": reactHooks, }, rules: { ... - "react-hooks/rules-of-hooks": "error", - "react-hooks/exhaustive-deps": "warn", "`@typescript-eslint/no-explicit-any`": "error",Ask on the web to confirm the exact rule set shipped in
reactHooks.configs.flat.recommendedfor the pinned^7.1.1version before applying, since presets evolve between minor releases.Also applies to: 60-61
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: e7a7cb19-8be6-4367-a72a-4f487fd490c9
⛔ Files ignored due to path filters (2)
web/mobile/src/styles/theme.generated.cssis excluded by!**/*.generated.*web/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (35)
.agents/skills/mobile-app-structure/SKILL.md.agents/skills/mobile-motion-patterns/SKILL.md.agents/skills/mobile-shadcn-conventions/SKILL.md.claude/skills/mobile-app-structure.claude/skills/mobile-motion-patterns.claude/skills/mobile-shadcn-conventionshosting/docker-compose/ee/docker-compose.dev.ymlhosting/docker-compose/oss/docker-compose.dev.ymlweb/ee/docker/Dockerfile.devweb/entrypoint.shweb/mobile/.gitignoreweb/mobile/AGENTS.mdweb/mobile/CLAUDE.mdweb/mobile/components.jsonweb/mobile/docker/Dockerfile.ghweb/mobile/eslint.config.mjsweb/mobile/next-env.d.tsweb/mobile/next.config.tsweb/mobile/package.jsonweb/mobile/postcss.config.mjsweb/mobile/public/.gitkeepweb/mobile/scripts/generate-shadcn-tokens.tsweb/mobile/src/components/ui/button.tsxweb/mobile/src/components/ui/skeleton.tsxweb/mobile/src/lib/motion/presets.tsweb/mobile/src/lib/utils.tsweb/mobile/src/pages/_app.tsxweb/mobile/src/pages/_document.tsxweb/mobile/src/pages/index.tsxweb/mobile/src/styles/globals.cssweb/mobile/tsconfig.jsonweb/oss/docker/Dockerfile.devweb/package.jsonweb/pnpm-workspace.yamlweb/turbo.json
| output: "standalone", | ||
| reactStrictMode: true, | ||
| pageExtensions: ["ts", "tsx"], | ||
| productionBrowserSourceMaps: true, |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== files =="
git ls-files | rg '(^|/)web/mobile/next.config\.ts$|(^|/)package\.json$|(^|/)pnpm-lock\.yaml$' || true
echo "== next config =="
cat -n web/mobile/next.config.ts
echo "== package scripts/dependencies relevant =="
if [ -f web/mobile/package.json ]; then cat -n web/mobile/package.json; fi
if [ -f web/package.json ]; then cat -n web/package.json; fi
echo "== source map settings in repo =="
rg -n "productionBrowserSourceMaps|next-sourcemap|source maps|sourcemaps|Production" web mobile .github 2>/dev/null || trueRepository: Agenta-AI/agenta
Length of output: 14562
Disable production browser source maps unless authorized tracking requires them.
Setting productionBrowserSourceMaps: true embeds source comments in client-ready JavaScript and creates source map artifacts, which can expose implementation details. If you need source maps for error tracking, upload them only to an authorized error tracking service and keep this option disabled for client delivery.
Proposed fix
- productionBrowserSourceMaps: true,
+ productionBrowserSourceMaps: false,📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| productionBrowserSourceMaps: true, | |
| productionBrowserSourceMaps: false, |
6534e92 to
39a672d
Compare
|
Verified all three against the code. Two were real and are fixed at the root; one I am not taking. 1. Greedy 2. Missing edition public dirs in the gh image — real, and it exposed a stack problem. The fix existed on the full branch but sat 28 commits later, which put it in PR 4 of this stack. So this PR really did ship an image whose container exits on start, and no reviewer of this PR could have known PR 4 repairs it. I fixed the cause rather than appending a patch: the repair is now folded into the commit that introduced the Dockerfile, so it never existed as a broken state. I audited the rest of the branch for the same shape (a later lane repairing an earlier lane) and this was the only case that shipped a genuinely broken state; the others are ordinary iteration on files an earlier lane introduced. 3. Also rebased onto current |
39a672d to
a8ad264
Compare
Railway Preview Environment
|
|
The four failing acceptance checks here are not from this PR. They are the repo-wide preview outage, and I traced the mechanism in case it helps whoever owns #5650. What fails: every acceptance job dies in Not specific to this stack. The Mechanism.
So the cloned environment runs with whatever the template environment carries, while Where a fix would go: either have the clone path apply the same secret set the legacy deploy does (the I have not changed any of it. It is not this stack's code, it is blocking other people's PRs too, and I cannot exercise Railway plumbing locally to verify a fix. |
|
@coderabbitai review |
✅ Action performedReview finished.
|
a8ad264 to
885738f
Compare
|
Production browser source maps: not taking here, and it is not a mobile decision.
If the repo wants source maps off in production, that is one change to both configs plus a decision about how errors get symbolicated afterward. That belongs to whoever owns the OSS build config, not to the PR that scaffolds a second Next app. Worth noting the finding is about disclosure, not a vulnerability: the maps expose the same TypeScript already published in this open-source repo. |
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 3a8594b8-41d7-4d50-8b96-5d72966c7c9d
⛔ Files ignored due to path filters (2)
web/mobile/src/styles/theme.generated.cssis excluded by!**/*.generated.*web/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (35)
.agents/skills/mobile-app-structure/SKILL.md.agents/skills/mobile-motion-patterns/SKILL.md.agents/skills/mobile-shadcn-conventions/SKILL.md.claude/skills/mobile-app-structure.claude/skills/mobile-motion-patterns.claude/skills/mobile-shadcn-conventionshosting/docker-compose/ee/docker-compose.dev.ymlhosting/docker-compose/oss/docker-compose.dev.ymlweb/ee/docker/Dockerfile.devweb/entrypoint.shweb/mobile/.gitignoreweb/mobile/AGENTS.mdweb/mobile/CLAUDE.mdweb/mobile/components.jsonweb/mobile/docker/Dockerfile.ghweb/mobile/eslint.config.mjsweb/mobile/next-env.d.tsweb/mobile/next.config.tsweb/mobile/package.jsonweb/mobile/postcss.config.mjsweb/mobile/public/.gitkeepweb/mobile/scripts/generate-shadcn-tokens.tsweb/mobile/src/components/ui/button.tsxweb/mobile/src/components/ui/skeleton.tsxweb/mobile/src/lib/motion/presets.tsweb/mobile/src/lib/utils.tsweb/mobile/src/pages/_app.tsxweb/mobile/src/pages/_document.tsxweb/mobile/src/pages/index.tsxweb/mobile/src/styles/globals.cssweb/mobile/tsconfig.jsonweb/oss/docker/Dockerfile.devweb/package.jsonweb/pnpm-workspace.yamlweb/turbo.json
🚧 Files skipped from review as they are similar to previous changes (31)
- web/entrypoint.sh
- web/mobile/CLAUDE.md
- .claude/skills/mobile-app-structure
- web/pnpm-workspace.yaml
- web/oss/docker/Dockerfile.dev
- web/turbo.json
- web/mobile/src/lib/utils.ts
- hosting/docker-compose/oss/docker-compose.dev.yml
- web/mobile/.gitignore
- web/mobile/package.json
- .agents/skills/mobile-motion-patterns/SKILL.md
- web/mobile/src/pages/_app.tsx
- web/mobile/src/pages/index.tsx
- web/package.json
- web/mobile/next.config.ts
- web/mobile/scripts/generate-shadcn-tokens.ts
- hosting/docker-compose/ee/docker-compose.dev.yml
- .agents/skills/mobile-shadcn-conventions/SKILL.md
- web/ee/docker/Dockerfile.dev
- .agents/skills/mobile-app-structure/SKILL.md
- .claude/skills/mobile-motion-patterns
- web/mobile/src/components/ui/skeleton.tsx
- web/mobile/docker/Dockerfile.gh
- web/mobile/src/components/ui/button.tsx
- web/mobile/postcss.config.mjs
- web/mobile/tsconfig.json
- .claude/skills/mobile-shadcn-conventions
- web/mobile/next-env.d.ts
- web/mobile/components.json
- web/mobile/src/lib/motion/presets.ts
- web/mobile/AGENTS.md
b0ee688 to
bfbd241
Compare
bfbd241 to
2be5bd8
Compare
2be5bd8 to
478de5b
Compare
The mobile app shipped with no icon assets and no icon link, so every tab and every add-to-home-screen shortcut fell back to the browser's placeholder globe. The .ico and the symbol are its OWN copies under public/assets, not a reference into the desktop app's /assets: this image is standalone, and borrowing that path would 404 the icon anywhere the desktop app is not deployed alongside it. The SVG gives a sharp mark on high-DPI tab strips and doubles as the apple-touch-icon, so a phone that installs the app to its home screen gets the brand rather than a screenshot. The hrefs carry the /m prefix because basePath is not applied to a bare href in _document, the same reason the __env.js script already writes it out.
Listing `inputs` for `@agenta/mobile#build` replaces Turbo's default file selection rather than adding to it, so the task hashed only `src`, `public` and four config files. `package.json` was not among them: a dependency bump changed nothing the cache could see, and CI would serve a build made against the old dependency tree. `$TURBO_DEFAULT$` restores every tracked file in the package as the base, with tests and markdown removed since neither reaches the bundle. Verified by hash: editing `web/mobile/package.json` moves `@agenta/mobile#build` from f7a97e16 to 9aa1f06c, where before it did not move at all.
478de5b to
0ea7f6a
Compare
1 participant
Context
Agenta has no mobile experience. The desktop app shows a "works better on larger screens" overlay on a phone, and the agent product now has a reason to be reachable from one: a cloud agent can pause for approval while you are away from your desk.
This is the first of a 12-PR stack that adds a minimal mobile web app at
/m. It contains the scaffold only. No screens, no data, nothing user-reachable yet: the app renders a placeholder, and the container that serves it does not start unless you ask for it.What this adds
A new
web/mobileworkspace package: a Next.js Pages Router app withbasePath: "/m"and standalone output, served behind Traefik at/malongside the desktop app. It shares the desktop's SuperTokens cookie session because it is the same origin.Styling comes from shadcn/ui on Tailwind v4, and the colors are not a second palette.
scripts/generate-shadcn-tokens.tsreads the existing source of truth,web/oss/src/styles/theme/palette.ts, and emitssrc/styles/theme.generated.css(committed, checked in CI). Change a color inpalette.tsand mobile follows.Three rules are lint-enforced rather than documented, because both are easy to violate by habit:
@ant-design/*, no Lexical. The app is shadcn-only.@/oss/*,@agenta/ossand@agenta/eeare banned; data comes from the@agenta/*packages.The dev compose service is opt-in behind
--with-mobile. It originally rode the existingwith-webprofile and started automatically, but a live run showed the second Next dev server pushing an 8GB Docker VM into OOM-killing the main web app's first Turbopack compile (dmesg-confirmednext-serverkills at ~4.5GB RSS). Running both dev servers wants a 12GB+ VM, so it does not start unless you opt in.Tests / notes
pnpm --filter @agenta/mobile types:checkandlint(which includes the generated-token freshness check) pass./mserves through Traefik while the desktop app keeps working at/.ghDockerfile is added here but not wired into the release build. That happens in a later PR in the stack, so this one cannot affect production images.entrypoint.shmirrors__env.jsinto the mobile app's public dir when that directory exists.What to QA
Nothing user-visible ships here. If you want to see it run:
bash ./hosting/docker-compose/run.sh --ee --dev --with-mobile --build, then openhttp://localhost/m. You get a placeholder page.http://localhost/in the same browser. The desktop app loads and your session still works.--with-mobile. Theweb-mobilecontainer does not start and the desktop app is unaffected.